Fix exception throwing inconsistency #17328
Conversation
❗ Release notes required
|
There was a problem hiding this comment.
Hmm.
AFAIU was not intentional this way, basically copypaste error.
While ArgumentOutOfRangeException is primarily used to indicate when an index argument is itself out of the range of valid indices, it does also seem to be used in the BCL where the bounds violation is the result of something more derived, e.g., here or here.
So invalidArgOutOfRange, which appears to be used mainly when an argument like count indirectly results in bounds being exceeded, seems like it could reasonably raise ArgumentOutOfRangeException instead of ArgumentException.
This doesn't seem to break anything.
This is not strictly true — there could be code out there like this whose behavior would now be different:
let f g =
try
g ()
with
| :? ArgumentOutOfRangeException -> [1]
| :? ArgumentException -> [2]
| _ -> [3]
// Currently returns [2], but would now return [1].
f (fun () -> [] |> List.skip 1)
|
I agree with the above, I made this change to have a discussion :) I understand this can change the behavior in theory, I wondered if it breaks something right away. We can keep things as they are of course, I am really just thinking thinking what to do with this thing in the other PR. My goal is just to minimize the code where it promises one exception and actually throws another one. |
|
@psfinaki please add release notes and mention there the possibility of breaking change. And if you could add a "Breaking Changes" section to https://github.com/dotnet/fsharp/blob/main/docs/release-notes/.FSharp.Core/8.0.400.md, that would be great. |
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
Another thing discovered when reviewing #17277.
AFAIU was not intentional this way, basically copypaste error.
This doesn't seem to break anything.